Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI: remove initial date from client counts #27816

Merged
merged 16 commits into from
Jul 31, 2024

Conversation

hashishaw
Copy link
Contributor

@hashishaw hashishaw commented Jul 19, 2024

Description

This PR updates the client count dashboard so that it sends the /activity request without start & end date parameters by default, which as of 1.18 will return the billing range by default. For the community edition, we will continue to not make the API call until there is an explicit date range set by the user, at which point query params are added to the URL.

CE experience
For community users, they must manually set a date range in order to see data. On dashboard page load, we do not initiate a request to the /activity endpoint until the user has adjusted the date range.
CE client count dashboard first load

We also don't allow the user to unset the query params for the date range:
CE client count date range editor when reset

Ent experience
In Enterprise, we do make a call to the /activity endpoint on page load, now without any query params so the backend will calculate the billing dates to use and return that data. In addition, the date range is pre-populated with the timestamp of the first month returned, and the end date from the response itself.
image

  • Ent tests pass

@hashishaw hashishaw added the ui label Jul 19, 2024
@hashishaw hashishaw added this to the 1.18.0-rc milestone Jul 19, 2024
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Jul 19, 2024
Copy link

github-actions bot commented Jul 19, 2024

CI Results:
All Go tests succeeded! ✅

@hashishaw hashishaw marked this pull request as ready for review July 19, 2024 18:23
@hashishaw hashishaw requested a review from a team as a code owner July 19, 2024 18:23
Copy link

github-actions bot commented Jul 19, 2024

Build Results:
All builds succeeded! ✅

if (!isNaN(millis)) {
timestamp = fromUnixTime(millis).toISOString();
}
// fallback to activity timestamp only if there was no query param
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for these comments!

Comment on lines +96 to +97
startTimestamp: this.paramOrResponseTimestamp(params?.start_time, activity?.byMonth[0]?.timestamp),
endTimestamp: this.paramOrResponseTimestamp(params?.end_time, activity?.endTime),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the new functionality is these params correspond to the range of data requested and the activity above (line 93) has start and end time parameters that correspond to the start_time and end_time from the /activity response?

I'm just thinking through the logic we have that lets users know "You have requested [start date] we only have data from [start of data]"

paramOrResponseTimestamp is helpfully named, I wonder if a comment above this function would help clarify the difference between selecting the first month timestamp vs the start/end params on the actual activity response 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, I can definitely add a comment with a description about what this does!

If query params were used, we want to set the start and end timestamps to match those, as you said, because the API start time matches the first month that data was returned. I found that if I just relied on the API timestamp responses, the message about only having data from [start of data] never showed up, because the timestamps matched. So, instead I had to default to the first month's timestamp if no query param was used, to ensure that the discrepancy message displayed when the data wasn't available for all months.

Copy link
Contributor

@hellobontempo hellobontempo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work on this ⭐ ⭐ ⭐ ⭐ ! 🚢

@hashishaw hashishaw merged commit 266ea69 into main Jul 31, 2024
31 checks passed
@hashishaw hashishaw deleted the ui/VAULT-27594/cc-remove-initial-date branch July 31, 2024 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants